-
Notifications
You must be signed in to change notification settings - Fork 97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
upload langchain b64 images to S3 #326
Conversation
^ Full example of what I mean
|
For the second TODO, created a ticket in Linear. Marking ready and merging |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Reviewed everything up to 3637364 in 1 minute and 32 seconds
More details
- Looked at
40
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. app-server/src/language_model/chat_message.rs:272
- Draft comment:
The regex pattern[a-zA-z]
has a typo. It should be[a-zA-Z]
to correctly match all alphabetic characters. This issue is present on lines 272 and 8. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The regex pattern[a-zA-z]
is technically incorrect - thez
should be uppercase in the range. However, in practice, MIME types are case-insensitive and typically use lowercase letters. The pattern will still match all valid MIME types like "image/jpeg", "image/png" etc. The error is more of a technical nitpick than a functional issue.
The regex could potentially miss some edge cases with uppercase MIME types. Also, since this is a code quality issue, maybe we should keep it for correctness.
While technically incorrect, the pattern will work correctly for all real-world use cases. The comment about line 8 is also incorrect, reducing credibility.
Delete the comment. The regex issue is minor and won't cause any practical problems. The incorrect reference to line 8 suggests the comment wasn't thoroughly validated.
Workflow ID: wflow_PeeRBO1mDKcXxmsx
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
https://python.langchain.com/docs/how_to/multimodal_inputs/
This is a WIP, because
RunnableParallel<raw>
for the example I tested with) and the root span (RunnableSequence
) also have the base64 payload in the attribute we parse as input, as part of stringified JSON. Ideally, we should replace that with link or at least remove that too, in order to reduce load on our DBImportant
Adds functionality to upload base64 encoded images in
ChatMessageContentPart::ImageUrl
to S3 and replace the URL with the S3 link.ChatMessageContentPart::ImageUrl
now checks if the URL is a base64 encoded image using a regex pattern.store_media()
.store_media()
inchat_message.rs
to handle base64 image URLs.regex
crate for pattern matching base64 image URLs.This description was created by
for 3637364. It will automatically update as commits are pushed.